Rework FixNullMotes patch #489
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This should fix rwmt#447, rendering rwmt#481 unnecessary. I've decided to still leave that code as an extra safety precaution, but it should be now safe to revert those changes. First, FixNullMotes patch was moved from generic "Patches" to "Determinism", as the patch itself changes some non-deterministic behavior. As fot the changes, this should fix issues with Bioferrite Harvester and Electroharvester cables not creating motes after switching maps. The current `FixNullMotes` patch would create a single mote per mote's `ThingDef.thingClass` and then reuse it. It ensured that the call to `MoteMaker.MakeStaticMote` never produced null, however the mote itself was more of a dummy mote (not initialized with any data). This caused issue with the cables that would periodically attempt to spawn motes. If the mote would not be possible to spawn (drawn off-screen/on a different map), the MP patch would return the cached mote. The cable connection comp would then keep that mote forever, as it can only discard it if the mote is null or destroyed (which would never happen with the dummy note). Because of this, our dummy mote never ended up being discarded by the comp making the cable never draw another mote. This PR ensures this doesn't happen by reworking "FixNullMotes" to actually spawn the motes, rather than returning a dummy mote. This is done by: - Setting `makeOffscreen` to true in a prefix, skipping current map check - Adding `& false` to `MoteCounter.Saturated` calls, allowing to spawn motes even if the mote counter is saturated The check for off-screen motes was left as-is, as that one should be deterministic and safe. Additional change call `Initialize` for both `Building_BioferriteHarvester` and `Building_Electroharvester`. Without it, the cables for those 2 aren't created after loading into the game (they're only created when rendered or a connection is added/removed). Since the cables aren't created the motes can't be created either, causing a discrepancy between players (unless they all had the cables created at the same time). A final note on the original patch - it seems that it was made to fix desyncs caused by motes spawned by `CompAbilityEffect_Waterskip` (and potentially other issues at the time, but I can't find any mention of those). However, this specific issue seems to no longer be relevant as the waterskip now uses flecks rather than motes.
SokyranTheDragon
added
biotech
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should fix #447, rendering #481 unnecessary. I've decided to still leave that code as an extra safety precaution, but it should be now safe to revert those changes.
As for the changes/fixes, this should fix issues with Bioferrite Harvester and Electroharvester cables not creating motes after switching maps. Additional minor change is that FixNullMotes patch was moved from generic "Patches" to "Determinism", as the patch itself changes some non-deterministic behavior.
The current
FixNullMotespatch would create a single mote per mote'sThingDef.thingClassand then reuse it. It ensured that the call toMoteMaker.MakeStaticMotenever produced null, however the mote itself was more of a dummy mote (not initialized with any data).This caused issue with the cables that would periodically attempt to spawn motes. If the mote would not be possible to spawn (drawn off-screen/on a different map), the MP patch would return the cached mote. The cable connection comp would then keep that mote forever, as it can only discard it if the mote is null or destroyed (which would never happen with the dummy note). Because of this, our dummy mote never ended up being discarded by the comp making the cable never draw another mote.
This PR ensures this doesn't happen by reworking "FixNullMotes" to actually spawn the motes, rather than returning a dummy mote. This is done by:
makeOffscreento true in a prefix, skipping current map check& falsetoMoteCounter.Saturatedcalls, allowing to spawn motes even if the mote counter is saturatedAdditional change is to call
Initializefor bothBuilding_BioferriteHarvesterandBuilding_ElectroharvesterinSpawnSetup, always creating cables for those buildings. Without it, the cables for those 2 aren't created after loading into the game (they're only created when rendered or a connection is added/removed). Since the cables aren't created the motes can't be created either, causing a discrepancy between players (unless they all had the cables created at the same time).A final note on the original patch - it seems that it was made to fix desyncs caused by motes spawned by
CompAbilityEffect_Waterskip(and potentially other issues at the time, but I can't find any mention of those). However, this specific issue seems to no longer be relevant as the waterskip now uses flecks rather than motes.